-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vite: Fix prebundling #19978
Vite: Fix prebundling #19978
Conversation
1dcdb51
to
8e929c8
Compare
This is mostly working. Building storybook succeeds, and |
@IanVS I'm still seeing this in the dev mode of storybook: Also see it in build-mode |
The CI error is weird because it's a peerDep and we add it to the project when initializing storybook:
How could the dep not be there? |
can confirm, it's not being added as a dep when running the sanbox command:
Need to debug this, why |
@IanVS I think I found the problem: storybook/code/lib/builder-vite/src/build.ts Lines 16 to 19 in 30cfe6c
conflicts with: // sandbox/lit-vite-default-ts/vite.config.ts
import { defineConfig } from 'vite';
export default defineConfig({
build: {
lib: {
entry: 'src/my-element.ts',
formats: ['es']
},
rollupOptions: {
external: /^lit/
}
}
}) Might not be getting merged correctly? Also there's this code: storybook/code/lib/builder-vite/src/plugins/code-generator-plugin.ts Lines 64 to 67 in 30cfe6c
Or maybe this:
Matches too much? I tried removing that bit of code from the sandbox's Another bit of information @IanVS the dev-mode is working fine, it's only a problem when building storybook. |
the |
… source of conflicts
emptyOutDir: false, // do not clean before running Vite build - Storybook has already added assets in there! | ||
sourcemap: true, | ||
}; | ||
config.build = mergeConfig(config, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could remove the mergeConfig
here, since we do not want to merge the build config with the user's build config.
Issue: N/A
@ndelangen noticed that
/sb-preview/runtime.mjs
was being bundled into the bundle created by vite for the iframe, rather than being externalized and requested from the static output folder as it should have been.What I did
This replaces the esbuild plugin,
@fal-works/esbuild-plugin-global-externals
with a rollup plugin that works more similarly to the webpack externals method. We can give it a list of packages which should be treated as globals, and the name of the global variable where they can be accessed.This also removes the re-exports of storybook packages from the vite frameworks, since they are not accessed from global scope instead. This essentially reverts #19216.
And I simplified
@storybook/preview
, since we no longer need to generate the ModuleInfo objects for the esbuild plugin. And we can export the mapping between package names and global variables, to use in both vite and webpack.How to test
CI. We should also check that pnpm still works correctly.